Defines TaggableCLArray by kaushikcfd · Pull Request #111 · inducer/arraycontext

kaushikcfd · 2021-10-20T02:37:40Z

Must go after Make pt.Array's axes taggable types pytato#170
Point the correct branches in requirements.txt.

inducer

Thanks! Some thoughts from a first scroll below.

inducer · 2022-01-15T00:17:46Z

Unsubscribing... @-mention or request review once it's ready for a look or needs attention.

kaushikcfd · 2022-01-15T02:51:06Z

Tests should pass once inducer/pyopencl#531 (or something equivalent) lands.

alexfikl · 2022-01-15T03:38:03Z

Any chance this works with pytential too? That has a very "interesting" combination of using array contexts and direct calls to pyopencl, which is easy to break.

Maybe worth adding to the CI?

kaushikcfd · 2022-01-15T03:49:57Z

and direct calls to pyopencl

🤢

Thanks, added a CI here: kaushikcfd/pytential#1. Will see if anything "interesting" pops up.

kaushikcfd · 2022-01-15T04:11:49Z

@alexfikl: Thanks for catching that.

===== 136 failed, 65 passed, 1 skipped, 385 warnings in 828.91s (0:13:48) ======

There are quite a few failures in pytential.

Probably not a smart thing to require this type. So there are a couple of options I can do here:

Fix pytential to not do this. (Unsure how involved this would be)
Just keep this limited to PytatoPyOpenCLArrayContext.

alexfikl · 2022-01-15T04:24:19Z

@alexfikl: Thanks for catching that.

===== 136 failed, 65 passed, 1 skipped, 385 warnings in 828.91s (0:13:48) ======

There are quite a few failures in pytential.

Not bad! 🚀

Probably not a smart thing to require this type. So there are a couple of options I can do here:
1. Fix pytential to not do this. (Unsure how involved this would be)
2. Just keep this limited to PytatoPyOpenCLArrayContext.

I imagine it would be quite a bit of work to get pytential to work. Besides calling pyopencl.array directly, it also calls loopy directly (without actx.call_loopy) and a few algorithms from pyopencl (ElementwiseKernel?).. so, yeah, a bit of work!

Would it be possible to just have the pyopencl array context handle both the tagged and untagged arrays?

kaushikcfd · 2022-01-15T15:58:47Z

I imagine it would be quite a bit of work to get pytential to work. Besides calling pyopencl.array directly, it also calls loopy directly (without actx.call_loopy) and a few algorithms from pyopencl (ElementwiseKernel?).. so, yeah, a bit of work!

Ah, thanks! Yep, I'll just go with not requiring TaggableCLArray in PyOpenCLArrayContext.

kaushikcfd · 2022-01-15T18:34:41Z

not requiring TaggableCLArray in PyOpenCLArrayContext.

Yep, doing that seems to fix the CIs in pytential as well! (Thanks!)

This PR is ready for review.

inducer

Thanks! Good to go after these fixes.

inducer · 2022-01-31T07:02:40Z

+                                  tags=tags, axes=axes)
+        else:
+            new_with_queue = super().copy(queue=queue)
+            return self.__class__(None, new_with_queue.shape,


Is the code duplication of the two constructor calls here avoidable? (Maybe by setting a base_instance variable?)

Indeed, using base_instance was helpful.

inducer · 2022-01-31T07:08:02Z

+    return TaggableCLArray(None, ary.shape,
+                           ary.dtype,
+                           allocator=ary.allocator,
+                           strides=ary.strides,
+                           data=ary.base_data,
+                           offset=ary.offset,
+                           events=ary.events, _fast=True,
+                           _context=ary.context,
+                           _queue=ary.queue, _size=ary.size,
+                           axes=axes,
+                           tags=tags)


I don't love the idea of hardcoding so much of the internal structure of cla.Array here. Is there a copy method in Array that we could use instead?

Not quite, see cla.Array.copy. But tried to avoid the duplication by invoking TaggableCLArray.copy. So there's only one source location where we rely on the structure on cl.array.Array.__init__'s signature.

inducer

Thanks! LGTM once these few concerns are addressed.

Co-authored-by: Andreas Klöckner <inform@tiker.net>

…rays

…ray as well

inducer · 2022-02-15T17:25:40Z

LGTM, thanks!

This was referenced Oct 20, 2021

Make pt.Array's axes taggable types inducer/pytato#170

Merged

Implements TaggableCLArray #110

Closed

kaushikcfd force-pushed the taggable_pt_array_axes branch from 1b97811 to a5a22b1 Compare November 1, 2021 05:09

kaushikcfd changed the title ~~Taggable pt array axes~~ Allowing taggable frozen PytatoArrayContext arrays and tagging pt arrays' axes Nov 1, 2021

kaushikcfd force-pushed the taggable_pt_array_axes branch from a5a22b1 to cb7eaee Compare November 1, 2021 18:47

kaushikcfd force-pushed the taggable_pt_array_axes branch from cb7eaee to 7e27d1b Compare November 18, 2021 05:55

kaushikcfd mentioned this pull request Nov 21, 2021

Add discretization entity tags to array axes inducer/grudge#188

Closed

1 task

kaushikcfd force-pushed the taggable_pt_array_axes branch 5 times, most recently from db03d5f to 3e9fa62 Compare January 14, 2022 01:07

kaushikcfd changed the title ~~Allowing taggable frozen PytatoArrayContext arrays and tagging pt arrays' axes~~ Defines TaggableCLArray Jan 14, 2022

kaushikcfd requested a review from inducer January 14, 2022 01:08

kaushikcfd marked this pull request as ready for review January 14, 2022 01:09

kaushikcfd force-pushed the taggable_pt_array_axes branch from 3e9fa62 to 09c3ad3 Compare January 14, 2022 01:24

inducer reviewed Jan 15, 2022

View reviewed changes

kaushikcfd force-pushed the taggable_pt_array_axes branch 2 times, most recently from 295f82d to 97cbed9 Compare January 15, 2022 02:48

kaushikcfd requested a review from inducer January 15, 2022 02:50

kaushikcfd force-pushed the taggable_pt_array_axes branch from 97cbed9 to f4d1eb4 Compare January 15, 2022 04:38

kaushikcfd force-pushed the taggable_pt_array_axes branch 2 times, most recently from d26faf5 to 9f4f766 Compare January 15, 2022 17:45

alexfikl mentioned this pull request Jan 15, 2022

Use arraycontext instead of pyopencl inducer/pytential#128

Merged

kaushikcfd force-pushed the taggable_pt_array_axes branch 2 times, most recently from c78083c to 3355f1c Compare January 21, 2022 16:43

inducer reviewed Jan 31, 2022

View reviewed changes

kaushikcfd added 2 commits February 8, 2022 13:07

recursively add tags to all arrays in the container

fee9ac5

tag pt arrays' axes

b74e236

kaushikcfd force-pushed the taggable_pt_array_axes branch 3 times, most recently from dc5c01b to f273e3d Compare February 8, 2022 20:31

kaushikcfd requested a review from inducer February 8, 2022 20:32

inducer approved these changes Feb 10, 2022

View reviewed changes

Comment thread arraycontext/impl/pyopencl/__init__.py

Comment thread arraycontext/impl/pyopencl/__init__.py

Comment thread arraycontext/impl/pytato/__init__.py

Comment thread arraycontext/impl/pytato/compile.py

kaushikcfd and others added 4 commits February 12, 2022 12:22

define TaggableCLArray

611cb22

Co-authored-by: Andreas Klöckner <inform@tiker.net>

make the frozen type of PytatoPyOpenCLArrayContext to be TaggableCLAr…

21c3882

…rays

adds tests specific to PytatoArrayContexts

cf58281

PyOpenCLArrayContext.make_einsum: use tagged tags for the output ar…

b8631d3

…ray as well

kaushikcfd force-pushed the taggable_pt_array_axes branch from f273e3d to b8631d3 Compare February 12, 2022 18:23

kaushikcfd mentioned this pull request Feb 12, 2022

Make pytato.Axis be a typing.Protocol instead of a concrete type. inducer/pytato#261

Open

kaushikcfd requested a review from inducer February 12, 2022 18:28

inducer merged commit b964a1b into inducer:main Feb 15, 2022

inducer mentioned this pull request Feb 21, 2022

Fix TaggableCLArray copy without queue #147

Merged

Conversation

kaushikcfd commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

inducer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

inducer commented Jan 15, 2022

Uh oh!

kaushikcfd commented Jan 15, 2022

Uh oh!

alexfikl commented Jan 15, 2022

Uh oh!

kaushikcfd commented Jan 15, 2022

Uh oh!

kaushikcfd commented Jan 15, 2022

Uh oh!

alexfikl commented Jan 15, 2022

Uh oh!

kaushikcfd commented Jan 15, 2022

Uh oh!

kaushikcfd commented Jan 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

inducer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

inducer Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

kaushikcfd Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

inducer Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

kaushikcfd Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

inducer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

inducer commented Feb 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kaushikcfd commented Oct 20, 2021 •

edited

Loading

kaushikcfd commented Jan 15, 2022 •

edited

Loading